Merge the get/download methods on the Source trait

Nothing currently implements the ability to more efficiently download a set of
packages at any one point in time, and the download/get distinction isn't really
used at all. We can always refactor later, but currently there's no benefit, nor
can it really be seen what the possible benefit is, so let's just merge these
two methods into one and have them operate on one id at a time.

diff --git a/src/cargo/core/registry.rs b/src/cargo/core/registry.rs
index f56aaa93a7ab3ed316027f8c5b81a74c6d605525..bd44d77877793cf28a7b8261c713ce7332b1c6ac 100644 (file)
--- a/src/cargo/core/registry.rs
+++ b/src/cargo/core/registry.rs
@@ -2,7 +2,7 @@ use std::collections::{HashSet, HashMap};
 
 use core::{Source, SourceId, SourceMap, Summary, Dependency, PackageId, Package};
 use core::PackageSet;
-use util::{CargoResult, ChainError, Config, human, profile};
+use util::{CargoResult, ChainError, Config, human, internal, profile};
 
 /// Source of information about a group of packages.
 ///
@@ -89,23 +89,14 @@ impl<'cfg> PackageRegistry<'cfg> {
                -> CargoResult<PackageSet<'cfg>> {
         trace!("getting packages; sources={}", self.sources.len());
 
-        // TODO: Only call source with package ID if the package came from the
-        // source
-        let mut ret = Vec::new();
-
-        for (_, source) in self.sources.sources_mut() {
-            try!(source.download(package_ids));
-            let packages = try!(source.get(package_ids));
-
-            ret.extend(packages.into_iter());
-        }
-
-        // TODO: Return earlier if fail
-        assert!(package_ids.len() == ret.len(),
-                "could not get packages from registry; ids={:?}; ret={:?}",
-                package_ids, ret);
+        let pkgs = try!(package_ids.iter().map(|id| {
+            let src = try!(self.sources.get_mut(id.source_id()).chain_error(|| {
+                internal(format!("failed to find a source listed for `{}`", id))
+            }));
+            src.download(id)
+        }).collect::<CargoResult<Vec<_>>>());
 
-        Ok(PackageSet::new(ret, self.sources))
+        Ok(PackageSet::new(pkgs, self.sources))
     }
 
     fn ensure_loaded(&mut self, namespace: &SourceId, kind: Kind) -> CargoResult<()> {

diff --git a/src/cargo/core/source.rs b/src/cargo/core/source.rs
index 8cc679858870f53e2c8d8339d682272fa15672c5..64779af37319505902eb31e35c7af9c39c3bf585 100644 (file)
--- a/src/cargo/core/source.rs
+++ b/src/cargo/core/source.rs
@@ -9,7 +9,7 @@ use rustc_serialize::{Decodable, Decoder, Encodable, Encoder};
 
 use url::Url;
 
-use core::{Summary, Package, PackageId, Registry, Dependency};
+use core::{Package, PackageId, Registry};
 use sources::{PathSource, GitSource, RegistrySource};
 use sources::git;
 use util::{human, Config, CargoResult, ToUrl};
@@ -24,13 +24,7 @@ pub trait Source: Registry {
 
     /// The download method fetches the full package for each name and
     /// version specified.
-    fn download(&mut self, packages: &[PackageId]) -> CargoResult<()>;
-
-    /// The get method returns the Path of each specified package on the
-    /// local file system. It assumes that `download` was already called,
-    /// and that the packages are already locally available on the file
-    /// system.
-    fn get(&self, packages: &[PackageId]) -> CargoResult<Vec<Package>>;
+    fn download(&mut self, package: &PackageId) -> CargoResult<Package>;
 
     /// Generates a unique string which represents the fingerprint of the
     /// current state of the source.

diff --git a/src/cargo/ops/cargo_install.rs b/src/cargo/ops/cargo_install.rs
index 36fdabaf49615f9731841bc60d532d13c9c12719..9bce345cb44b1c00cda4a883e9a51638352dc8dd 100644 (file)
--- a/src/cargo/ops/cargo_install.rs
+++ b/src/cargo/ops/cargo_install.rs
@@ -130,9 +130,8 @@ fn select_pkg<'a, T>(mut source: T,
             let deps = try!(source.query(&dep));
             match deps.iter().map(|p| p.package_id()).max() {
                 Some(pkgid) => {
-                    try!(source.download(&[pkgid.clone()]));
-                    Ok((try!(source.get(&[pkgid.clone()])).remove(0),
-                        Box::new(source)))
+                    let pkg = try!(source.download(pkgid));
+                    Ok((pkg, Box::new(source)))
                 }
                 None => {
                     let vers_info = vers.map(|v| format!(" with version `{}`", v))

diff --git a/src/cargo/sources/git/source.rs b/src/cargo/sources/git/source.rs
index 4a66b1c3159202c919eff5de890634ca6b208d6e..0cdb02f25d33ca882c728f8b705ba4b776500a9a 100644 (file)
--- a/src/cargo/sources/git/source.rs
+++ b/src/cargo/sources/git/source.rs
@@ -187,16 +187,12 @@ impl<'cfg> Source for GitSource<'cfg> {
         self.path_source.as_mut().unwrap().update()
     }
 
-    fn download(&mut self, _: &[PackageId]) -> CargoResult<()> {
-        // TODO: assert! that the PackageId is contained by the source
-        Ok(())
-    }
-
-    fn get(&self, ids: &[PackageId]) -> CargoResult<Vec<Package>> {
-        trace!("getting packages for package ids `{:?}` from `{:?}`", ids,
-             self.remote);
-        self.path_source.as_ref().expect("BUG: update() must be called \
-                                          before get()").get(ids)
+    fn download(&mut self, id: &PackageId) -> CargoResult<Package> {
+        trace!("getting packages for package id `{}` from `{:?}`", id,
+               self.remote);
+        self.path_source.as_mut()
+                        .expect("BUG: update() must be called before get()")
+                        .download(id)
     }
 
     fn fingerprint(&self, _pkg: &Package) -> CargoResult<String> {

diff --git a/src/cargo/sources/path.rs b/src/cargo/sources/path.rs
index 206a9f0a400d7a7f6a21af6094735498b61148bd..26373c7ba7e530b76e81616521721d378465ec66 100644 (file)
--- a/src/cargo/sources/path.rs
+++ b/src/cargo/sources/path.rs
@@ -302,18 +302,13 @@ impl<'cfg> Source for PathSource<'cfg> {
         Ok(())
     }
 
-    fn download(&mut self, _: &[PackageId])  -> CargoResult<()>{
-        // TODO: assert! that the PackageId is contained by the source
-        Ok(())
-    }
-
-    fn get(&self, ids: &[PackageId]) -> CargoResult<Vec<Package>> {
-        trace!("getting packages; ids={:?}", ids);
+    fn download(&mut self, id: &PackageId) -> CargoResult<Package> {
+        trace!("getting packages; id={}", id);
 
-        Ok(self.packages.iter()
-           .filter(|pkg| ids.iter().any(|id| pkg.package_id() == id))
-           .cloned()
-           .collect())
+        let pkg = self.packages.iter().find(|pkg| pkg.package_id() == id);
+        pkg.cloned().ok_or_else(|| {
+            internal(format!("failed to find {} in path source", id))
+        })
     }
 
     fn fingerprint(&self, pkg: &Package) -> CargoResult<String> {

diff --git a/src/cargo/sources/registry.rs b/src/cargo/sources/registry.rs
index 7e0ad0558e3582836a8b394ff10e7fe727546155..1c68475e057c805b8fd2797f6b23ff99ac55eba8 100644 (file)
--- a/src/cargo/sources/registry.rs
+++ b/src/cargo/sources/registry.rs
@@ -187,7 +187,6 @@ pub struct RegistrySource<'cfg> {
     src_path: PathBuf,
     config: &'cfg Config,
     handle: Option<http::Handle>,
-    sources: HashMap<PackageId, PathSource<'cfg>>,
     hashes: HashMap<(String, String), String>, // (name, vers) => cksum
     cache: HashMap<String, Vec<(Summary, bool)>>,
     updated: bool,
@@ -239,7 +238,6 @@ impl<'cfg> RegistrySource<'cfg> {
             config: config,
             source_id: source_id.clone(),
             handle: None,
-            sources: HashMap::new(),
             hashes: HashMap::new(),
             cache: HashMap::new(),
             updated: false,
@@ -537,37 +535,24 @@ impl<'cfg> Source for RegistrySource<'cfg> {
         Ok(())
     }
 
-    fn download(&mut self, packages: &[PackageId]) -> CargoResult<()> {
+    fn download(&mut self, package: &PackageId) -> CargoResult<Package> {
         let config = try!(self.config());
         let url = try!(config.dl.to_url().map_err(internal));
-        for package in packages.iter() {
-            if self.source_id != *package.source_id() { continue }
-            if self.sources.contains_key(package) { continue }
-
-            let mut url = url.clone();
-            url.path_mut().unwrap().push(package.name().to_string());
-            url.path_mut().unwrap().push(package.version().to_string());
-            url.path_mut().unwrap().push("download".to_string());
-            let path = try!(self.download_package(package, &url).chain_error(|| {
-                internal(format!("failed to download package `{}` from {}",
-                                 package, url))
-            }));
-            let path = try!(self.unpack_package(package, path).chain_error(|| {
-                internal(format!("failed to unpack package `{}`", package))
-            }));
-            let mut src = PathSource::new(&path, &self.source_id, self.config);
-            try!(src.update());
-            self.sources.insert(package.clone(), src);
-        }
-        Ok(())
-    }
+        let mut url = url.clone();
+        url.path_mut().unwrap().push(package.name().to_string());
+        url.path_mut().unwrap().push(package.version().to_string());
+        url.path_mut().unwrap().push("download".to_string());
+        let path = try!(self.download_package(package, &url).chain_error(|| {
+            internal(format!("failed to download package `{}` from {}",
+                             package, url))
+        }));
+        let path = try!(self.unpack_package(package, path).chain_error(|| {
+            internal(format!("failed to unpack package `{}`", package))
+        }));
 
-    fn get(&self, packages: &[PackageId]) -> CargoResult<Vec<Package>> {
-        let mut ret = Vec::new();
-        for src in self.sources.values() {
-            ret.extend(try!(src.get(packages)).into_iter());
-        }
-        Ok(ret)
+        let mut src = PathSource::new(&path, &self.source_id, self.config);
+        try!(src.update());
+        src.download(package)
     }
 
     fn fingerprint(&self, pkg: &Package) -> CargoResult<String> {